Remove unsafety from sys/unsupported and add deny(unsafe_op_in_unsafe_fn). #77722

m-ou-se · 2020-10-08T21:12:04Z

Replacing UnsafeCells by a Cells simplifies things and makes the mutex and rwlock implementations safe. Other than that, only unsafety in strlen() contained unsafe code.

@rustbot modify labels: +F-unsafe-block-in-unsafe-fn +C-cleanup

Replacing the UnsafeCell by a Cell simplifies things and makes it all safe.

Replacing the UnsafeCell by a Cell makes it all safe.

rust-highfive · 2020-10-08T21:12:07Z

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

library/std/src/sys/unsupported/rwlock.rs

Co-authored-by: Joshua Nelson <[email protected]>

Mark-Simulacrum · 2020-10-13T13:25:06Z

Could you add a comment on both introduced Cells that they're for platforms without threads so there's no need for synchronization? The UnsafeCell would've wanted that comment too, I feel.

It seems a bit unfortunate that we implement Sync for the Mutex/RwLock in sys/unsupported -- do we really need to do that? It's obviously untrue... it seems like maybe ideally Rust would provide a cfg(no_threads) or whatever for us.

m-ou-se · 2020-10-13T13:27:49Z

Sure. Will add a comment.

It's obviously untrue...

Well they're safe to share between all the threads your program can have on those platforms: just the one main thread. ^^'

Without Sync, you couldn't use one in a static like on other platforms.

m-ou-se · 2020-10-13T13:30:43Z

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

Mark-Simulacrum · 2020-10-13T14:01:05Z

Yes, I mean that it would be better if (for example) the compiler removed Sync bounds on statics, for example, on single-threaded platforms. But that would require lang approval and design :)

@bors r+

bors · 2020-10-13T14:03:47Z

📌 Commit b26aa5d has been approved by Mark-Simulacrum

@rustbot

…ported-locks, r=Mark-Simulacrum Remove unsafety from sys/unsupported and add deny(unsafe_op_in_unsafe_fn). Replacing `UnsafeCell`s by a `Cell`s simplifies things and makes the mutex and rwlock implementations safe. Other than that, only unsafety in strlen() contained unsafe code. @rustbot modify labels: +F-unsafe-block-in-unsafe-fn +C-cleanup

JohnTitor · 2020-10-13T16:43:47Z

Failed in https://github.com/rust-lang-ci/rust/runs/1248784774:

warning: dropping unsupported crate type `dylib` for target `wasm32-unknown-unknown`

error: unnecessary `unsafe` block
  --> library/std/src/sys/wasm/../unsupported/common.rs:43:5
   |
43 |     unsafe {
   |     ^^^^^^ unnecessary `unsafe` block
   |
   = note: `-D unused-unsafe` implied by `-D warnings`

error: aborting due to previous error; 1 warning emitted

[RUSTC-TIMING] std test:false 2.295
error: could not compile `std`

@bors r-

m-ou-se · 2020-10-13T16:50:53Z

Ah, apparently sys/wasm takes some stuff directly from sys/unsupported. Then sys/unsupported/mod.rs gets bypassed and the deny(unsafe_op_in_unsafe_fn) attribute does not apply.

m-ou-se · 2020-10-13T16:55:37Z

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

Checked locally that std now compiles for wasm32-unknown-unknown. I only applied the attribute to the one mondule where it was necessary. Looks like sys/wasm will be applying the attribute to everything soon as well: #74477

…m too.

jyn514 · 2020-10-13T17:20:57Z

@bors r=Mark-Simulacrum rollup=iffy

bors · 2020-10-13T17:20:59Z

📌 Commit af414dc has been approved by Mark-Simulacrum

bors · 2020-10-13T19:07:13Z

⌛ Testing commit af414dc with merge 772eaeb38d5203c0efb95201b52b2bbaa780c734...

bors · 2020-10-13T20:20:22Z

💔 Test failed - checks-actions

JohnTitor · 2020-10-13T20:26:27Z

@bors retry
Seems the failure is due to GitHub outage (it's resolved now): https://www.githubstatus.com/incidents/nq2s4gt7xs1w

m-ou-se · 2020-10-13T20:32:05Z

Ah, thanks! Was just about to post a comment because I didn't understand what failed.

By the way, looks like all the relevant platforms for this PR passed their tests, so rollup should be fine now. ^^

Rollup of 14 pull requests Successful merges: - rust-lang#77239 (Enable building Cargo for aarch64-apple-darwin) - rust-lang#77569 (BTreeMap: type-specific variants of node_as_mut and cast_unchecked) - rust-lang#77719 (Remove unnecessary rustc_const_stable attributes.) - rust-lang#77722 (Remove unsafety from sys/unsupported and add deny(unsafe_op_in_unsafe_fn).) - rust-lang#77725 (Add regression issue template) - rust-lang#77776 ( Give an error when running `x.py test --stage 0 src/test/ui`) - rust-lang#77786 (Mention rustdoc in `x.py setup`) - rust-lang#77825 (`min_const_generics` diagnostics improvements) - rust-lang#77868 (Include `llvm-dis`, `llc` and `opt` in `llvm-tools-preview` component) - rust-lang#77884 (Use Option::unwrap_or instead of open-coding it) - rust-lang#77886 (Replace trivial bool matches with the `matches!` macro) - rust-lang#77892 (Replace absolute paths with relative ones) - rust-lang#77895 (Include aarch64-apple-darwin in the dist manifests) - rust-lang#77909 (bootstrap: set correct path for the build-manifest binary) Failed merges: - rust-lang#77902 (Include aarch64-pc-windows-msvc in the dist manifests) r? `@ghost`

m-ou-se added 2 commits October 8, 2020 23:08

Remove unsafety from unsupported/mutex.rs by using a Cell.

c25f69a

Replacing the UnsafeCell by a Cell simplifies things and makes it all safe.

Remove unsafety from unsupported/rwlosck.rs by using a Cell.

3d192ac

Replacing the UnsafeCell by a Cell makes it all safe.

rustbot added C-cleanup Category: PRs that clean code up or issues documenting cleanup. F-unsafe-block-in-unsafe-fn RFC #2585 labels Oct 8, 2020

rust-highfive assigned Mark-Simulacrum Oct 8, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 8, 2020

Apply deny(unsafe_op_in_unsafe_fn) to all of sys/unsupported.

f4e8842

m-ou-se changed the title ~~Remove unsafety from unsupported/{mutex,rwlock}.rs by using a Cell.~~ Remove unsafety from sys/unsupported and add deny(unsafe_op_in_unsafe_fn). Oct 8, 2020

m-ou-se mentioned this pull request Oct 8, 2020

#![deny(unsafe_op_in_unsafe_fn)] in libstd #73904

Closed

22 tasks

jyn514 added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Oct 8, 2020

jyn514 reviewed Oct 8, 2020

View reviewed changes

library/std/src/sys/unsupported/rwlock.rs Outdated Show resolved Hide resolved

Assert state in sys/unsupported's RwLock::write_unlock.

f1c3edb

Co-authored-by: Joshua Nelson <[email protected]>

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 13, 2020

Add note about using cells in the locks on the 'unsupported' platform.

b26aa5d

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 13, 2020

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 13, 2020

JohnTitor mentioned this pull request Oct 13, 2020

Rollup of 10 pull requests #77903

Closed

bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Oct 13, 2020

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 13, 2020

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 13, 2020

Deny unsafe_op_in_unsafe_fn for unsupported/common.rs through sys/was…

af414dc

…m too.

m-ou-se force-pushed the safe-unsupported-locks branch from f92b36a to af414dc Compare October 13, 2020 16:56

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 13, 2020

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 13, 2020

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 13, 2020

JohnTitor mentioned this pull request Oct 13, 2020

Rollup of 14 pull requests #77917

Merged

bors merged commit cc5a1aa into rust-lang:master Oct 14, 2020

rustbot added this to the 1.49.0 milestone Oct 14, 2020

Remove unsafety from sys/unsupported and add deny(unsafe_op_in_unsafe_fn). #77722

Remove unsafety from sys/unsupported and add deny(unsafe_op_in_unsafe_fn). #77722

Uh oh!

Conversation

m-ou-se commented Oct 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Oct 8, 2020

Uh oh!

Uh oh!

Mark-Simulacrum commented Oct 13, 2020

Uh oh!

m-ou-se commented Oct 13, 2020

Uh oh!

m-ou-se commented Oct 13, 2020

Uh oh!

Mark-Simulacrum commented Oct 13, 2020

Uh oh!

bors commented Oct 13, 2020

Uh oh!

JohnTitor commented Oct 13, 2020

Uh oh!

m-ou-se commented Oct 13, 2020

Uh oh!

m-ou-se commented Oct 13, 2020

Uh oh!

jyn514 commented Oct 13, 2020

Uh oh!

bors commented Oct 13, 2020

Uh oh!

bors commented Oct 13, 2020

Uh oh!

bors commented Oct 13, 2020

Uh oh!

JohnTitor commented Oct 13, 2020

Uh oh!

m-ou-se commented Oct 13, 2020

Uh oh!

Uh oh!

m-ou-se commented Oct 8, 2020 •

edited

Loading